Skip to content

SDK-108 Route registerToken through OfflineRequestProcessor for network-failure retry#1068

Merged
sumeruchat merged 4 commits into
masterfrom
fix/SDK-108-register-token-offline-retry
Jun 2, 2026
Merged

SDK-108 Route registerToken through OfflineRequestProcessor for network-failure retry#1068
sumeruchat merged 4 commits into
masterfrom
fix/SDK-108-register-token-offline-retry

Conversation

@sumeruchat

Copy link
Copy Markdown
Contributor

What

Brings registerToken to feature parity with disablePush (shipped under SDK-297): the registration request is now persisted to the offline task queue and replayed when the network returns, instead of being dropped fire-once on transient network failures. Customer-visible symptom this fixes: device never gets registered for push if registerForRemoteNotifications is called while the device is offline.

Jira: https://iterable.atlassian.net/browse/SDK-108

Changes

  • RequestProcessorProtocol.swift — add register(registerTokenInfo:notificationsEnabled:onSuccess:onFailure:) to the shared protocol so sendUsingRequestProcessor can pick offline vs online at call time. Add RequestIdentifier.registerToken (sibling to the existing disableDevice identifier).
  • RequestHandler.swift — resolve notificationStateProvider.isNotificationsEnabled first, then route through sendUsingRequestProcessor with the resolved bool. This snapshots notificationsEnabled at call time (parallel to SDK-297's UserIdentitySnapshot), so a request queued offline and replayed later uses the bool as it was when the caller invoked register — replay does not re-query notification state.
  • OfflineRequestProcessor.swift — implements the new register(...) using the existing RequestCreator.createRegisterTokenRequest(...) + sendIterableRequest(...) pattern. Drop-in mirror of disableDeviceForCurrentUser.
  • OnlineRequestProcessor.swift — public register(...) now takes notificationsEnabled: Bool directly (provider resolution moved up to RequestHandler); the redundant private helper is folded into the public method; uses RequestIdentifier.registerToken consistently.

Impact

  • Breaking changes: None. Public IterableAPI / InternalIterableAPI register(token:onSuccess:onFailure:) surface and Obj-C selectors are unchanged.
  • Dependencies: None.
  • Performance: Negligible. Same number of network calls in the success path; offline path now persists one row in the existing IterableTaskScheduler CoreData queue instead of dropping the request.
  • Snapshot semantics: explicitly tested — notificationsEnabled is frozen at call time. If notification permissions change between schedule and replay, the replayed request still carries the original bool. This matches the SDK-297 precedent and is the desired behavior for queued events.

Testing

How to test:

  1. Run ./agent_build.sh (succeeds) and ./agent_test.sh (601 unit / 12 notification-extension / 92 offline-events, 0 failures).
  2. New testOfflineRegisterTokenReplaysWithExpectedBody — confirms an offline-mode register persists and replays with the expected body.
  3. New testOfflineRegisterTokenRetriesAfterNetworkFailureWithSnapshottedNotificationsEnabled — sends a register, fails the first network attempt, mutates MockNotificationStateProvider.enabled from true → false, asserts both the failed first send and the replayed second send carry notificationsEnabled: true (the snapshotted value) and the task is removed from the queue after the successful replay.
  4. Manual sanity: with the app in airplane mode, call registerForRemoteNotifications; turn airplane mode off; observe one register-device-token request fired with the original notification-enabled bool.

…rk-failure retry

Brings registerToken to feature parity with disablePush (shipped under
SDK-297): the registration request is now persisted to the offline task
queue and replayed when the network returns, instead of being dropped
fire-once on transient network failures.

Implementation mirrors the SDK-297 disableDevice retrofit:
- Adds `register(...)` to `RequestProcessorProtocol` (taking the
  already-resolved `notificationsEnabled: Bool`, not the async
  notification-state provider) so both processors can implement it and
  `sendUsingRequestProcessor` can pick offline vs online at call time.
- Adds `RequestIdentifier.registerToken` (sibling to the existing
  `disableDevice` identifier).
- Implements `register(...)` in `OfflineRequestProcessor` using the
  existing `RequestCreator.createRegisterTokenRequest(...)` +
  `sendIterableRequest(...)` pattern.
- Updates `OnlineRequestProcessor.register(...)` to conform to the new
  protocol signature and consume the shared identifier; the existing
  private register helper is folded into the public one.

The notification-state wrinkle (parallel to SDK-297's
`UserIdentitySnapshot`): `notificationStateProvider.isNotificationsEnabled`
is now resolved inside `RequestHandler.register` BEFORE routing to a
processor. The resolved bool is captured into the offline task body at
schedule time, so a request queued offline and replayed later carries
the bool as it was at call time — replay does NOT re-query notification
state.

Public IterableAPI / InternalIterableAPI surface and Obj-C selectors are
unchanged. Tests in `offline-events-tests` cover (a) offline replay with
the expected body, and (b) the snapshot semantic — a
`MockNotificationStateProvider` flipped from `true` to `false` after
register is called still yields `notificationsEnabled: true` on the
replayed request.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
onSuccess: onSuccess,
onFailure: onFailure)
notificationStateProvider.isNotificationsEnabled { notificationsEnabled in
_ = self.sendUsingRequestProcessor { processor in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On master, register went straight to OnlineRequestProcessor and built the body synchronously after isNotificationsEnabled, so setCurrentUser read auth essentially at call time. After this PR, the build runs inside sendUsingRequestProcessor's deferred closure (after canSchedule() + a queue hop), and createRegisterTokenRequest still reads live authProvider.auth rather than a captured UserIdentitySnapshot like disableDeviceForCurrentUser does. Intentional, or should register mirror the SDK-297 snapshot pattern?

@sumeruchat sumeruchat Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mirrored the snapshot pattern in fb361ff. register now captures UserIdentitySnapshot at call time (before the async notification hop) and threads it into createRegisterTokenRequest, same as disableDeviceForCurrentUser. Tests cover offline-replay and online-fallback keeping the call-time identity. 36/0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional UserIdentitySnapshot? on six signatures + a forked branch in RequestCreator feels heavy for a race that only exists in RequestHandler.register's async hop. The disableDevice symmetry is also partial, those tasks survive handleLogout(), register doesn't. Can we fix it where the gap actually is instead of threading a parameter through every layer below it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified in eed546e. Dropped the cross-layer snapshot threading: capture call-time auth once in register, carry it on RegisterTokenInfo.auth, and RequestCreator resolves it through the existing single body path. ApiClient, both processors, and the protocol no longer carry the param. You were right on handleLogout, register tasks are purged on logout (only disableDevice is preserved), so the full symmetry was not needed. Kept a small identity guard in register because dropping it regresses onFailure on the offline no-user path. 36/0.

Address review: register now captures a UserIdentitySnapshot synchronously
before the async notification-state callback and deferred processor
selection, mirroring disableDeviceForCurrentUser. This ensures the
register request targets the user who was current at call time rather than
whoever live auth points to when the deferred/offline build resolves.

The snapshot threads through both the online and offline paths into
RequestCreator.createRegisterTokenRequest, which applies it (and derives
preferUserId from the .userId case) instead of reading live auth. Callers
that pass no snapshot keep the previous live-auth behavior and the
auth-missing guard.

Tests: register at call time then mutate/clear identity before the deferred
callback resolves; assert both offline-replay and online-fallback requests
still carry the call-time identity and preferUserId.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.21053% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.42%. Comparing base (62a33d3) to head (86ac36e).

Files with missing lines Patch % Lines
...k/Internal/api-client/Request/RequestHandler.swift 68.42% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1068      +/-   ##
==========================================
+ Coverage   71.31%   71.42%   +0.10%     
==========================================
  Files         112      113       +1     
  Lines        9389     9410      +21     
==========================================
+ Hits         6696     6721      +25     
+ Misses       2693     2689       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Drop the cross-layer UserIdentitySnapshot threading. Capture call-time auth
once in RequestHandler.register and carry it on RegisterTokenInfo.auth;
RequestCreator resolves registerTokenInfo.auth ?? self.auth through the
existing single body path. Reverts the snapshot parameter on ApiClient, the
processors, and the protocol.

Register offline tasks are purged on logout (only disableDevice is
preserved), so the full disableDevice snapshot symmetry was unnecessary.
Keep an upfront identity guard in register: dropping it would regress
onFailure on the offline no-user path, since the callback only attaches
after a task is scheduled.

Build green; offline RequestHandlerTests 36/0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r-token-offline-retry

* origin/master:
  SDK-300 Add OnSuccess/OnFailure handlers to logoutUser (#1069)
  SDK-469 Fix CodeQL string length conflation in deep link check (#1071)
  SDK-493: Prepare for Release 6.7.2 (#1070)
@sumeruchat sumeruchat merged commit 2f85293 into master Jun 2, 2026
15 checks passed
@sumeruchat sumeruchat deleted the fix/SDK-108-register-token-offline-retry branch June 2, 2026 13:54
sumeruchat added a commit that referenced this pull request Jun 2, 2026
…ush-async-await

* origin/master:
  SDK-108 Route registerToken through OfflineRequestProcessor for network-failure retry (#1068)
  SDK-300 Add OnSuccess/OnFailure handlers to logoutUser (#1069)
  SDK-469 Fix CodeQL string length conflation in deep link check (#1071)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants